Skip to content

Fix arm64: LD1 missing immediate operand #1628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
ekilmer wants to merge 3 commits into from
Closed

Fix arm64: LD1 missing immediate operand #1628

ekilmer wants to merge 3 commits into from

Conversation

@ekilmer
Copy link
Contributor

@ekilmer ekilmer commented May 8, 2020

Fixes #1627

Not sure if this is the best way to fix this, but it seems to work

@aquynh
Copy link
Collaborator

aquynh commented May 9, 2020 •
edited
Loading

did you check with the latest LLVM 10, to see if they have this issue?

@ekilmer
Copy link
Contributor Author

ekilmer commented May 9, 2020 •
edited
Loading

I did not. Apologies, since I'm not too familiar with the workflow for fixing things like this, but I think this is what you want to see?

$ llvm-mc --version
LLVM (http://llvm.org/):
  LLVM version 10.0.0
  Optimized build.
  Default target: x86_64-unknown-linux-gnu

$ echo "0xe0 0x73 0xdf 0x0c" | llvm-mc --disassemble --show-encoding --show-inst-operands --show-inst --arch=aarch64
        .text
        ld1     { v0.8b }, [sp], #8     // encoding: [0xe0,0x73,0xdf,0x0c]
                                        // <MCInst #2099 LD1Onev8b_POST
                                        //  <MCOperand Reg:5>
                                        //  <MCOperand Reg:41>
                                        //  <MCOperand Reg:5>
                                        //  <MCOperand Reg:8>>

So, if I'm understanding that output correctly, it looks like the immediate is missing between SP and XZR?

//  <MCOperand Reg:5>  # AArch64_SP
//  <MCOperand Reg:41> # AArch64_D0
//  <MCOperand Reg:5>  # AArch64_SP
//  <MCOperand Reg:8>  # AArch64_XZR

Or it's something else.

Maybe the immediate is implied, since I can only get { v0.8b } matching #8 and same with { v0.16b } matching with #16. The MCInst also has 8 or 16 in the name.

$ echo "ld1     { v0.16b }, [sp], #16" | llvm-mc --assemble --show-encoding --show-inst-operands --show-inst --arch=aarch64
        .text
<stdin>:1:1: note: parsed instruction: ['ld1', <vectorlist 121 >, '[', <register 5>, ']', 16]
ld1     { v0.16b }, [sp], #16
^
        ld1     { v0.16b }, [sp], #16   // encoding: [0xe0,0x73,0xdf,0x4c]
                                        // <MCInst #2087 LD1Onev16b_POST
                                        //  <MCOperand Reg:5>
                                        //  <MCOperand Reg:121>
                                        //  <MCOperand Reg:5>
                                        //  <MCOperand Reg:8>>

Where // <MCOperand Reg:121> is AArch64_Q0

Here's the manual for the instruction https://developer.arm.com/docs/ddi0596/e/simd-and-floating-point-instructions-alphabetic-order/ld1-multiple-structures-load-multiple-single-element-structures-to-one-two-three-or-four-registers

🤷

@aquynh
Copy link
Collaborator

aquynh commented May 9, 2020

i just pushed a fix, please confirm.

@ekilmer
Copy link
Contributor Author

ekilmer commented May 9, 2020

I think it only fixed a few variants. This one still doesn't work

./cstool -d arm64 '\xe0\xa3\xdf\x0c'
 0  e0 a3 df 0c  ld1    {v0.8b, v1.8b}, [sp], #16
        ID: 279 (ld1)
        op_count: 3
                operands[0].type: REG = v0
                operands[0].access: READ | WRITE
                        Vector Arrangement Specifier: 0x2
                operands[1].type: REG = v1
                operands[1].access: READ | WRITE
                        Vector Arrangement Specifier: 0x2
                operands[2].type: MEM
                        operands[2].mem.base: REG = sp
                operands[2].access: READ
        Write-back: True
        Registers read: v0 v1 sp
        Registers modified: v0 v1 sp
        Groups: neon

@ekilmer
Copy link
Contributor Author

ekilmer commented May 9, 2020

@aquynh I added the rest of the variants in this commit b180fab and reverted my previous commit fix.

This passes my tool's tests, but I don't think we test the following:

AArch64_LD1i8_POST
AArch64_LD1i16_POST
AArch64_LD1i32_POST
AArch64_LD1i64_POST

@aquynh
Copy link
Collaborator

aquynh commented May 10, 2020

please make pull req

@ekilmer ekilmer mentioned this pull request May 10, 2020
Merged
@ekilmer
Copy link
Contributor Author

ekilmer commented May 10, 2020

Made new PR #1632 . Closing this one now.

@ekilmer ekilmer closed this May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ekilmer @aquynh